feat: named RetryCurve API for switching retry regimes at runtime - #68
Open
tanderson-ld wants to merge 3 commits into
Open
feat: named RetryCurve API for switching retry regimes at runtime#68tanderson-ld wants to merge 3 commits into
tanderson-ld wants to merge 3 commits into
Conversation
Introduces a RetryCurve opaque handle. Callers construct curves via NewRetryCurve(options...), designate them at subscribe time via StreamOptionDefaultRetryCurve / StreamOptionRegisterRetryCurve, and switch between them at runtime via Stream.ActivateCurve. Enables SDKs to run a multi-regime retry policy (e.g., a normal regime + an extended regime for auth failures) while keeping the library's single-regime timing path intact for legacy callers. Overlay resolution walks (active-curve spec -> effective-default spec -> hard-coded fallbacks), evaluated lazily at delay-computation time. Per-curve formula counter n is retained across activations. Healthy-operation reset zeros all curves' formula counters and reverts to the effective default; it does not clear base-delay overrides (matches SSE spec's "reconnection time is set until updated"). SSE `retry:` field is honored per HTML5 semantics: the stream read loop updates every registered curve's base-delay override. Values above 1 hour are clamped per RETRY spec section 1.11.4 (new MaxServerDirectedRetryDelay constant). Clamping happens in milliseconds before the multiplication by time.Millisecond so extreme wire values cannot overflow the Duration. Internal changes: - Widened backoffStrategy.applyBackoff and jitterStrategy.applyJitter to accept per-call maxDelay / ratio so a single strategy instance can serve multiple curves. Math bodies unchanged from the pre-existing library. - Renamed internal SetBaseDelay to ApplyRetryTime; it now iterates all registered curves. Legacy stream options (StreamOptionInitialRetry / UseBackoff / UseJitter / RetryResetInterval) continue to work unchanged; when no explicit RetryCurve is provided they synthesize the effective default. Refs SDK-2788.
tanderson-ld
commented
Aug 6, 2026
| pub := ev.(*publication) | ||
| if pub.Retry() > 0 { | ||
| stream.retryDelay.SetBaseDelay(time.Duration(pub.Retry()) * time.Millisecond) | ||
| stream.retryDelay.ApplyRetryTime(clampServerDirectedRetry(pub.Retry())) |
Author
There was a problem hiding this comment.
For reviewers: renamed, existing name was misleading, even on main this did more than set base delay.
tanderson-ld
commented
Aug 6, 2026
|
|
||
| var delayedEvent eventOrComment | ||
| jitterStrategy := newDefaultJitter(0.5, 0) | ||
| jitterStrategy := newDefaultJitter(0) |
Author
There was a problem hiding this comment.
For reviewers: Jitter is now passed as a param to the strategy at jitter application time.
tanderson-ld
commented
Aug 6, 2026
| type backoffStrategy interface { | ||
| applyBackoff(baseDelay time.Duration, retryCount int) time.Duration | ||
| applyBackoff(baseDelay time.Duration, retryCount int, maxDelay time.Duration) time.Duration | ||
| } |
Author
There was a problem hiding this comment.
For reviewers: backoffStrategy and jitterStrategy are internal only interfaces. The max and jitter are now properties of the retry curve and not fixed in the strategy.
tanderson-ld
commented
Aug 6, 2026
| } | ||
|
|
||
| type defaultJitterStrategy struct { | ||
| ratio float64 |
Author
There was a problem hiding this comment.
For reviewers: jitter ratio moved to the retry curve to support cases of different jitters in different sitatuions.
tanderson-ld
commented
Aug 6, 2026
| // streamOptions. | ||
| func newRetryDelayStrategyFromOptions(opts *streamOptions, randSeed int64) *retryDelayStrategy { | ||
| // Resolve the effective default curve. | ||
| effectiveDefault := opts.defaultRetryCurve |
Author
There was a problem hiding this comment.
For reviewers: if a default curve was not provided, we will make a default curve from the old stream options in order to not be a breaking change.
tanderson-ld
commented
Aug 6, 2026
| func (r *retryDelayStrategy) SetBaseDelay(baseDelay time.Duration) { | ||
| // Does NOT reset the newly-activated curve's retryCount — each curve's counter | ||
| // retains its progression across activations. Does NOT touch any curve's | ||
| // baseDelayOverride. |
Author
There was a problem hiding this comment.
For reviewers: the baseDelayOverride is set via the server directed retry: event.
- Rename `max` parameter on RetryCurveMaxDelay to `maxDelay` (revive redefines-builtin-id: `max` shadows the Go 1.21 built-in). - Add `//nolint:unused // used only in tests` to activeCurve, matching the existing convention on hasJitter. - Wrap the applyBackoff signature and the three firstNonNil calls in resolveCurveProperties across multiple lines (lll: 120-char limit). No logic changes. `make lint` and `go test ./...` both clean locally.
tanderson-ld
marked this pull request as ready for review
August 6, 2026 13:58
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
RetryCurveAPI so a caller can register multiple retry-timing curves on a single stream and switch between them at runtime viaStream.ActivateCurve. Enables consumers to run a multi-regime retry policy (e.g., a normal regime + an extended regime for auth failures) while keeping the library's single-regime timing path intact for existing callers.Marked draft for API/design socialization before dependent SDK work builds on it.
API additions
NewRetryCurve(options ...RetryCurveOption) *RetryCurve— construct an opaque handle.RetryCurveBaseDelay(d),RetryCurveMaxDelay(d),RetryCurveJitter(r)— curve options.StreamOptionDefaultRetryCurve(curve)— designate as the stream's effective default.StreamOptionRegisterRetryCurve(curve)— register as an additional switchable curve.Stream.ActivateCurve(curve *RetryCurve)— switch the currently-active curve at runtime.DefaultCurve— package-level sentinel meaning "revert to the effective default."MaxServerDirectedRetryDelay = time.Hour— clamp ceiling for the SSEretry:field.Legacy stream options (
StreamOptionInitialRetry,StreamOptionUseBackoff,StreamOptionUseJitter,StreamOptionRetryResetInterval) continue to work unchanged; when no explicitRetryCurveis provided they synthesize the effective default.Semantics
active-curve.spec→effective-default.spec→ hard-coded fallback. Curve specs are immutable.n. Each registered curve tracks its own backoff-formula counter. Progression is retained across activations, so rapid oscillation between regimes preserves each regime's state.elapsed >= resetInterval, zeros every curve'snand revertsactiveto the effective default. Does NOT clear server-directed base-delay overrides (matches HTML5 SSE spec's "reconnection time is set until updated").retry:field. Updates every registered curve's base-delay override (stream-wide per HTML5). Never touches any curve's declaredmaxDelayceiling.retry:values aboveMaxServerDirectedRetryDelayare clamped, in milliseconds before thetime.Millisecondmultiplication, so extreme int64 wire values cannot overflow theDuration.Internal notes for reviewers
backoffStrategy.applyBackoffandjitterStrategy.applyJitterinterfaces were widened to accept per-callmaxDelay/ratio. Math bodies are unchanged from the pre-existing library; only the parameter source moved from receiver fields to method args, so one strategy instance can serve multiple curves.SetBaseDelayrenamed toApplyRetryTime; it now iterates all registered curves.Test plan
go test ./...).make contract-tests— "All tests passed").Refs SDK-2788.